add functions to allow conversions between d and tth, and d and q #154
Conversation
| """ | ||
| epsilon = 1e-10 | ||
| d = np.asarray(self.on_d[0]) | ||
| d = np.where(np.abs(d) < epsilon, d + epsilon, d) |
There was a problem hiding this comment.
Added d to a small number to avoid dividing by 0, which would cause q to become some arbitrary large number (as commented later in the test function). Not sure if this is a good way to address this problem - do we want to just raise error in these cases?
| r""" | ||
| Helper function to convert q to d using :math:`d = \frac{2 \pi}{q}` | ||
|
|
||
| adds a small value (epsilon = 1e-10) to `q` where `q` is close to zero to avoid division by zero |
There was a problem hiding this comment.
this seems like not a great way to handle this. Let's think about this.
| """ | ||
| epsilon = 1e-10 | ||
| q = np.asarray(self.on_q[0]) | ||
| q = np.where(np.abs(q) < epsilon, q + epsilon, q) |
There was a problem hiding this comment.
let's think a bit how best to do this. Also, we may need to think if we want the d array to be so non-linear. How are we handling the interpolation when we go from q to tth? We can use that as inspiration.
| """ | ||
| epsilon = 1e-10 | ||
| d = np.asarray(self.on_d[0]) | ||
| d = np.where(np.abs(d) < epsilon, d + epsilon, d) |
tests/test_diffraction_objects.py
Outdated
| @@ -231,6 +231,85 @@ def test_diffraction_objects_equality(inputs1, inputs2, expected): | |||
| assert (diffraction_object1 == diffraction_object2) == expected | |||
|
|
|||
|
|
|||
| def test_q_to_tth(): | |||
| actual = Diffraction_object(wavelength=0.71) | |||
There was a problem hiding this comment.
let's have a quick convo about what we want to test. Just one well behavied number is good, but any edge cases? IIn which case which ones? then maybe we can figure out a way to create the inputs once and reuse them to test all the transforms together to save time? That may also be a mistake.
|
@sbillinge I edited the codes based on our previous conversation, it's ready for review. The history for this PR is getting a bit messy, and I also saw Bob's PR for change the name for Diffraction object -- I can probably start a new PR for this after his PR is merged? |
There was a problem hiding this comment.
Let's focus on the tests and iterate those so we get the behavior we want. Basically if Q goes out of bounds (tth > 180) something has gone wrong and I htink we should raise and exception alerting this to user and asking if the wavelength is correctly specified.
| """ | ||
| q = self.on_q[0] | ||
| q = np.asarray(q) | ||
| wavelength = float(self.wavelength) | ||
| pre_factor = wavelength / (4 * np.pi) | ||
| return np.rad2deg(2.0 * np.arcsin(q * pre_factor)) | ||
| # limit q * pre_factor to 1 to avoid invalid input to arcsin |
There was a problem hiding this comment.
I am not sure about this. If arcsin_value is greater than 1 something bad has gone wrong. We probably want to raise an exception and let the user know. It probably means they have specified a wrong wavelength of something.
| q = np.where(np.abs(q) < epsilon, q + epsilon, q) | ||
| return (2 * np.pi) / q | ||
| d = np.where(q != 0, (2 * np.pi) / q, np.inf) | ||
| d = np.minimum(d, DMAX) |
There was a problem hiding this comment.
This is not very readable. Also, I guess it can end up with a bunch of values of D that are the same. I think there is a better way to do this maybe. Also, how does the user overrid DMAX if they want?
There was a problem hiding this comment.
The user can use insert_scattering_quantity to overrid or directly set the attributes. But I agree with you that this might be problematic. Also the appropriate limit seems a bit different for different wavelength. Maybe it's better to set d = 2 * previous_d if q = 0?
e.g. q = [0, 1] => d = [2pi, 4pi]
q = [0, 2] => d = [pi, 2pi]
| @@ -10,6 +10,8 @@ | |||
| DQUANTITIES = ["d", "dspace"] | |||
| XQUANTITIES = ANGLEQUANTITIES + DQUANTITIES + QQUANTITIES | |||
| XUNITS = ["degrees", "radians", "rad", "deg", "inv_angs", "inv_nm", "nm-1", "A-1"] | |||
| QMAX = 40 | |||
There was a problem hiding this comment.
this is too low probably.
tests/test_diffraction_objects.py
Outdated
| actual_tth = actual.q_to_tth() | ||
| expected_tth = [0, 30] | ||
| # expected tth values are 2 * arcsin(q) |
There was a problem hiding this comment.
this also doesn't make any sense to me. I don't think we want this behavior.
|
no need to review - just for saving progress |
|
@yucongalicechen I will close this since it is replaced by those issues (and future PRs). Thanks for doing this. |
closes #146
Added functions to convert between tth and d, and d and q. Modified the function set_all_arrays so that when we create a new diffraction object, we have arrays on all xtypes. Added tests for these modified functions.
@sbillinge ready for review